WebExtension (Firefox) compatibility for chromeIPass #595
Conversation
browser-poyfill.min.js comes from https://github.com/mozilla/webextension-polyfill master branch 77af627d0 with following two pull requests cherry-picked on top: mozilla/webextension-polyfill#11 mozilla/webextension-polyfill#22
|
I appreciate your work, I look forward to more updates and reviewing these
changes with you. Thank you.
…On Thu, Feb 16, 2017 at 3:26 AM Angus Gratton ***@***.***> wrote:
As discussed in #563 <#563>. Allows
Electrolysis (multi-process windows) feature to function in recent Firefox,
and also provides a path for support once XUL-based addons are disabled in
Firefox.
Support is not quite as tightly integrated as it was with passifox (ie
there's no integration to Firefox's own "Fill Password" functions any
more), but it seems very usable all the same.
Still a work in progress:
- Changed all chrome.xxx APIs to use browser.. Some are implementated
in Firefox and work as-is, but a few (such as
chrome.extension.sendMessage() are not. Seemed cleaner to change them
all over.
- Add Mozilla's webextension-polyfill for Chrome compatibility. Needed
to cherry-pick some PRs before things came good (details in commit
message). The basics seem to work.
- Thoroughly test for broken Chrome functionality. I suspect there
will still be some things which don't work.
- Firefox doesn't support webRequest.onAuthRequired (yet?) so can't
fill HTTP auth popups from Chromeipass. Need to at least re-enable for
Chrome, and hide the relevant Settings on Firefox.
- Haven't confirmed extension persistent storage (association to
keypass, etc.) is working properly in FF. I think this is because I can
only load the extension temporarily, so it's removing localStorage
whenever I restart the browser. Firefox production builds don't allow
unsigned addons at all any more, except temporarily.
- Has a Chrome Web Store link in the Settings when running on Firefox
- jQuery ("cIPJQ") seems to have some issues on FF that may be
breaking functionality.
- Detecting password fields for generation of new passwords doesn't
yet work on FF, I think related to the jQuery errors but I'm not sure.
Also, I don't know if this would be a good time to change the name? Or
even if you're interested in adding this support at all - given it's a
fairly major change to what seems like a stable Chrome extension.
------------------------------
You can view, comment on, or merge this pull request online at:
#595
Commit Summary
- chromeipass: Initial WebExtension support - can fill passwords in
Firefox
- chromeipass: Add browser-polyfill for Chrome compatibility (part
tested)
File Changes
- *M* chromeipass/background/browserAction.js
<https://github.com/pfn/passifox/pull/595/files#diff-0> (8)
- *M* chromeipass/background/event.js
<https://github.com/pfn/passifox/pull/595/files#diff-1> (9)
- *M* chromeipass/background/init.js
<https://github.com/pfn/passifox/pull/595/files#diff-2> (36)
- *M* chromeipass/background/keepass.js
<https://github.com/pfn/passifox/pull/595/files#diff-3> (2)
- *M* chromeipass/background/page.js
<https://github.com/pfn/passifox/pull/595/files#diff-4> (8)
- *A* chromeipass/browser-polyfill.min.js
<https://github.com/pfn/passifox/pull/595/files#diff-5> (9)
- *M* chromeipass/chromeipass.js
<https://github.com/pfn/passifox/pull/595/files#diff-6> (76)
- *M* chromeipass/manifest.json
<https://github.com/pfn/passifox/pull/595/files#diff-7> (11)
- *M* chromeipass/options/options.html
<https://github.com/pfn/passifox/pull/595/files#diff-8> (1)
- *M* chromeipass/options/options.js
<https://github.com/pfn/passifox/pull/595/files#diff-9> (33)
- *M* chromeipass/popups/popup.html
<https://github.com/pfn/passifox/pull/595/files#diff-10> (1)
- *M* chromeipass/popups/popup.js
<https://github.com/pfn/passifox/pull/595/files#diff-11> (18)
- *M* chromeipass/popups/popup_functions.js
<https://github.com/pfn/passifox/pull/595/files#diff-12> (17)
- *M* chromeipass/popups/popup_httpauth.js
<https://github.com/pfn/passifox/pull/595/files#diff-13> (9)
- *M* chromeipass/popups/popup_login.html
<https://github.com/pfn/passifox/pull/595/files#diff-14> (1)
- *M* chromeipass/popups/popup_login.js
<https://github.com/pfn/passifox/pull/595/files#diff-15> (8)
- *M* chromeipass/popups/popup_multiple-fields.html
<https://github.com/pfn/passifox/pull/595/files#diff-16> (1)
- *M* chromeipass/popups/popup_remember.html
<https://github.com/pfn/passifox/pull/595/files#diff-17> (1)
- *M* chromeipass/popups/popup_remember.js
<https://github.com/pfn/passifox/pull/595/files#diff-18> (28)
- *M* documentation/chromeIPass.md
<https://github.com/pfn/passifox/pull/595/files#diff-19> (9)
Patch Links:
- https://github.com/pfn/passifox/pull/595.patch
- https://github.com/pfn/passifox/pull/595.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#595>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAfQxeaClLKFbf3CRVeYQiRlr1mXkNjcks5rdDJZgaJpZM4MC5QR>
.
|
|
Thanks @pfn :) I'm hoping to work on it in bits and pieces over the next little while. My day job is embedded development so playing with Javascript again is a breath of fresh air. At the same time, if anyone else is reading this and wants to jump in and tackle any of the items mentioned above, please be my guest. |
chrome-extension:// doesn't seem to have a static equivalent, so find at runtime using .getURL() and build a style tag. Bit ickier. :/
Firefox doesn't allow clipboard copy on the background tab. Verified still working in Chromium.
projectgus
changed the title
…n Firefox Uses the cross-browser webextensions API for onAuthRequested, so it may also work in Opera or Edge(?), and could start working some day in Firefox if Mozilla adds support.
|
Coming along pretty well. Decided I may not need to fix the jQuery errors I'm seeing in FF console, they don't appear to have any bearing on behaviour. |
|
Could I help you with some testing? Don't have much experience with browser extensions but willing to try/learn. |
Sure, that'd be great! To install a development version in Firefox:
Usual caveats about development versions, trusting with your data, etc. apply. :) As the addon is unsigned, it will uninstall when Firefox exits and you'll need to reload it. So far I've needed to re-pair it with KeepassXC each time this happens (there are some options in about:config, "extensions.webextensions.keepStorageOnUninstall" and "extensions.webextensions.keepUuidOnUninstall" but this has not been enough to make the association persist.) Still haven't looked to see if this is a bug in the addon or a security limitation of Firefox that can possibly be bypassed another way. My current status on this is that I started yak shaving by adding support for context menu accelerator keys to firefox WebExtensions. If you want to dig deeper, the Mozilla docs for WebExtensions are good: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/ |
|
So far works great. Have so far only noticed the bug you are working on atm, which seems to be a FF issue. I feel like your patch is ready to be pulled, while the compatibility is added to FF. Or would you rather wait till you have that fixed too? Remaining bugs (bullet 3) will be found when more people start using this and start reporting them, and 5 should be fixed when this gets signed... |
|
I signed a version for testing, if anyone wants to give this a go. Just drag and drop the xpi into your Firefox window, should install and persist across sessions. chromeipass-2.8.0-an+fx.xpi.zip I can confirm that number 5 is done when using this signed and packaged addon. |
Excellent, thank you!
I think I generally agree. There's only bug I've seen now, sometimes if you click "Fill User & Pass" in the contextual menu then a debugging window pops up due to a fatal error. Doesn't seem to happen every time. Other than this, the contextual menu titles look a bit weird in Firefox but it's easy enough to open a separate issue to track this, until Firefox supports them. Angus |
|
Works fine for me on FF 52.0.1 with GNOME on kernel 4.10.5. The only issue I see is that Google's login (split over two pages) does not auto-fill on the first page, i.e. I have to manually add in my user name, then on the second page I have to manually click 're-scan credentials fields' for it to work. |
|
multi-page logins will likely never be supported.
…On Mon, Mar 27, 2017 at 1:30 AM Taijian ***@***.***> wrote:
Works fine for me on FF 52.0.1 with GNOME on kernel 4.10.5.
The only issue I see is that Google's login (split over two pages) does
not auto-fill on the first page, i.e. I have to manually add in my user
name, then on the second page I have to manually click 're-scan credentials
fields' for it to work.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#595 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAfQxeR7oYuX4sThJvIUPPyzyy7PFaIXks5rp3OQgaJpZM4MC5QR>
.
|
|
@pfn What work is still needed to get this PR merged? |
|
@Ivan0xFF From my point of view I'd be happy for this to be merged now, and I'll open an issue to track the contextual menu thing. If it stays open for a few more days then over the weekend I was hoping to try and track down the occasional error I was getting when selecting "Fill User & Pass". Although it seems like it may require some special combination of events to reproduce, as I've only seen it once or twice. So I don't think it's a showstopper. In the end it's all up to @pfn as maintainer, of course. :) |
projectgus
changed the title
|
(I've checked "test for broken Chrome functionality" off my list as I've put Chromium through its major paces with this PR. I think if there are more subtle bugs they'll only be found once a larger number of users start using it, unfortunately.) |
|
Thanks, I'll look at getting this reviewed and merged soon.
…On Wed, Mar 29, 2017 at 3:11 PM Angus Gratton ***@***.***> wrote:
@Ivan0xFF <https://github.com/Ivan0xFF> From my point of view I'd be
happy for this to be merged now, and I'll open an issue to track the
contextual menu thing.
If it stays open for a few more days then over the weekend I was hoping to
try and track down the occasional error I was getting when selecting "Fill
User & Pass". Although it seems like it may require some special
combination of events to reproduce, as I've only seen it once or twice. So
I don't think it's a showstopper.
In the end it's all up to @pfn <https://github.com/pfn> as maintainer, of
course. :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#595 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAfQxZJlUsqtqDo6F3TD5ibu8hbQbhLiks5rqtcJgaJpZM4MC5QR>
.
|
|
I just tried @Ivan0xFF's version. It works with all of my daily sites. Awesome! Just noticed that there are quite a few errors like this in the terminal: Is it harmless or a potential sign for a bug? |
|
There's another issue: in the legacy Firefox addon, there's usually only one KeepassHTTP confirmation dialog for an password field. With this version, a KeepassHTTP confirmation dialog jumps out whenever a web page loses focus and gets focus again. This is somewhat annoying. |
@varjolintu I only use Chromium a little bit, but from what I recall it seems like this is an issue there as well? Or is it only an issue when running the WebExtension on Firefox? |
|
@projectgus i'm fine with merging my changes into this PR, not sure if it will help with moving things along at all. |
|
Cool. Well, this PR branch is now level with your 2.8.5 release tag. I left out the most recent commit which changes README URLs, as if this PR is to be merged then we don't want that. The other change which we may not want here is the new addon id. But I'm not going to bother changing it back until we know what @pfn plans to do. (BTW, @smorks, I think the idea of you taking over as maintainer provided this works for @pfn.) |
|
@projectgus It happens with both browsers. I have an idea how to solve it but haven't tested it yet. Basically you could monitor any hidden divs that contain forms and do a re-detect when any of those hidden div's are visible again. @smorks & @projectgus I really like these changes you've been making. If it's ok to you I would like to merge some of those to keepassxc-browser (and give you a credit of course). |
|
@varjolintu If it's not Firefox-specific then probably best to open a separate issue to discuss it. I agree it would be nice to fix if possible (I do a lot of clicking on the chromeipass icon and saying "Redetect credential fields"). Regarding merging my changes, I have no problems with them being merged anywhere they are useful. |
|
@varjolintu i'm fine with you merging my changes too. |
|
Will this ever get merged? |
|
I made a PR to projectgus/passifox#1 which solves an issue with HTTP auth with Chromium based browsers. It's not pretty but works as a workaround until there's a better solution. |
|
Hmm. How about using MutationObserver to detect DOM changes? https://stackoverflow.com/a/11546242 May still need a timer to rate limit and avoid a notable performance impact (ie reset a ~100ms timer each time the DOM changes, so 100ms after the last DOM change event the extension scans for login boxes.) EDIT: Actually, given you can filter the events you care about it may be possible to filter so you only get an event if a password field changes. I don't know if that would include the parent div's visibility changing, though. BTW, @varjolintu, sorry I haven't looked at your other PR content yet. Plan to do so this weekend. |
|
@projectgus I already closed the pull request. @smorks did a better job implementing the same fix :) |
|
The recent release of Firefox 57.0b1 makes the WebExtension-compatible version of Passifox all that much more urgent! Thanks for your continued work on this. Edit: Just signed and installed it myself and it is working great! |
|
@hackel For those of us not familiar with signing/installing manually, would you mind providing a short description of the required steps, or at least a link? |
|
Track this project for a WebExtension fork of passifox/chromeipass: https://github.com/smorks/keepasshttp-connector |
|
And now it's on AMO: https://addons.mozilla.org/en-US/firefox/addon/keepasshttp-connector/ |
|
Will passifox (on AMO) be updated to webextension in the future? Or will this "kepasshttp-connector" be the version to use on 57+? |
|
Firefox 57 has now been released, are there any plans to get this shipped soon? |
|
@RealOrangeOne Loot at the comment above from @Ivan0xFF. The firefox compatible fork is already on AMO: https://addons.mozilla.org/en-US/firefox/addon/keepasshttp-connector/ I'm using it every day and works great :) |
|
Does it work with 52 ESR?
…On November 14, 2017 7:17:05 PM EST, Daniel Kraus ***@***.***> wrote:
@RealOrangeOne Loot at the comment above from @Ivan0xFF. The firefox
compatible fork is already on AMO:
https://addons.mozilla.org/en-US/firefox/addon/keepasshttp-connector/
I'm using it every day and works great :)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#595 (comment)
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
|
The fork works, but its the chrome version, which doesnt integrate nicely with the builtin password manager. Really hope that feature stays, assuming the |
|
No that feature does not work. It is not yet exposed by the WebExtensions API. |
|
Is the KeePassHTTP Connector add-on safe for users to use? I'm new to KeePass and am hoping that some kind of fix for Firefox Quantum integration that is secure is available. |
|
@esat7 It is safe. |
As discussed in #563. Allows Electrolysis (multi-process windows) feature to function in recent Firefox, and also provides a path for support once XUL-based addons are disabled in Firefox.
Support is not quite as tightly integrated as it was with passifox (ie there's no integration to Firefox's own "Fill Password" functions any more), but it seems very usable all the same.
Still a work in progress:
chrome.xxxAPIs to usebrowser.. Some chrome APIs are implementated in Firefox and work as-is, but a few (such aschrome.extension.sendMessage()are not. Seemed cleaner to change them all over.browser.xxx. Needed to cherry-pick some PRs before things came good (details in commit message). The basics seem to work.webRequest.onAuthRequired(yet?) so can't fill HTTP auth popups from Chromeipass. Need to at least re-enable for Chrome, and hide the relevant Settings on Firefox.localStoragewhenever I restart the browser. Firefox production builds don't allow unsigned addons at all any more, except temporarily.Also, I don't know if this would be a good time to change the name? Or even if you're interested in adding this support at all - given it's a fairly major change to what seems like a stable Chrome extension.